Skip to content

Fix the entry points left unadjusted from the template - #21

Open
peterbjohnson wants to merge 1 commit into
mainfrom
fix_boilerplate_entrypoints
Open

Fix the entry points left unadjusted from the template#21
peterbjohnson wants to merge 1 commit into
mainfrom
fix_boilerplate_entrypoints

Conversation

@peterbjohnson

Copy link
Copy Markdown
Member

Closes #14

Four defects inherited from the boilerplate. None affected the deployed eval path, but each misled anyone working on the repo.

Approach

Tests first, in dev_test.py (new, following the existing <module>_test.py convention) and a rewritten preview_test.py. Before the change:

12 failed, 1 passed

The one pass was the old preview test, which asserted only that a preview key existed and was not None. That is why the sympy echo survived this long.

1. dev.py failed immediately

The command the README pointed at:

python -m evaluation_function.dev "<response>" "<answer>"
AttributeError: 'dict' object has no attribute 'to_dict'

Two faults. It called .to_dict() on a value that is a plain dict, and it passed its arguments as (answer, response) while the signature is (response, answer). The argument swap is the nastier one: it does not raise, it just silently exchanges "missing" and "extra" in the feedback. It now prints the result as JSON and passes the arguments the right way round.

2. preview.py was the sympy placeholder

It echoed the response back under a sympy key, which is meaningless for MIDI. It now reports what the system actually read:

14 notes, 8.0 s, C4 to A4.

An audio path is named but deliberately not transcribed, because the preview runs while the student is still working and transcription takes seconds:

Audio recording practice.wav, transcribed on submission.

The platform's Preview type carries only sympy and feedback, so the summary goes in feedback.

I implemented this rather than removing the preview registration, which #12 offered as the alternative. Removing it would mean command: preview starts returning an error, and I would not want to change what the platform sees on the strength of a maintenance pass. Say the word if you would rather it went.

3. Wrong return annotation in evaluation.py

Annotated -> Result, referring to lf_toolkit's Result class, which this function has never returned. Worth noting this is not merely cosmetic: actually returning a Result would change the output, because that class renders feedback by joining items with <br>, which would mangle the newline-separated message this function produces. The dict is correct, so the annotation is what changes. Three tests in section 8 now pin the returned shape.

4. Healthcheck failed inside the container

command: healthcheck runs the test suite, and evaluation_test.py reads its bulk cases from data/, which the image did not carry:

FileNotFoundError: /app/data/longMIDIsequence.json

.dockerignore excluded the whole data/ directory, so simply adding a COPY failed the build outright. It now admits that one fixture and nothing else, so datasets and recordings that land in data/ still stay out of the image. The fixture load also tolerates the file being absent, rather than failing the whole module at import and taking every other test down with it.

Verified in a container built from this branch, with #12's backend fix layered on so it would boot on arm64:

Before After
tests_passed false true
errors evaluation_function.evaluation_test none
successes 1 70

Only the 56 KB fixture lands in /app/data.

Verification

Check Result
New tests, before 12 failed, 1 passed
New tests, after 13 passed
Full suite 88 passed
CI lint gate (E9,F63,F7,F82) 0
Container healthcheck 70 passed, 0 errors

Note on test file placement

dev_test.py is a new file, but it mirrors dev.py exactly as preview_test.py mirrors preview.py, so it follows the convention already in the repo rather than adding another ad-hoc one. The return-shape tests went into section 8 of evaluation_test.py, where the other evaluation_function integration tests live, deliberately avoiding a new numbered section that would collide with #13's section 11.

🤖 Generated with Claude Code

Four defects inherited from the boilerplate. None affected the deployed
eval path, but each misled anyone working on the repo.

dev.py: the documented command failed immediately. It called .to_dict() on
a value that is a plain dict, and passed its arguments as (answer,
response) while the function's signature is (response, answer), which
silently swaps "missing" and "extra" in the feedback. It now prints the
result as JSON and passes the arguments the right way round.

preview.py: returned the response echoed back under a "sympy" key, which
is meaningless for MIDI. It now reports what was read: note count,
duration and pitch range. An audio path is named but deliberately not
transcribed, because the preview runs while the student is still working
and transcription takes seconds. The platform's Preview type carries only
"sympy" and "feedback", so the summary goes in "feedback".

evaluation.py: annotated as returning lf_toolkit's Result class, which it
has never done. Returning one would change the output, because that class
renders feedback by joining items with "<br>", which would mangle the
newline-separated message. The dict is correct, so the annotation is what
changes.

healthcheck: the command runs the test suite, and evaluation_test.py reads
its bulk cases from data/, which the image did not carry. Copy the fixture
in, and tolerate its absence rather than failing the whole module at
import. .dockerignore excluded the directory outright, so it now admits
that one file and nothing else.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@ada-jz7125 ada-jz7125 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are text assertions in preview_test.py, but I think this keyword assertion is acceptable as long as the preview is not changed completely

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Leftover boilerplate: dev.py, preview.py, healthcheck and return annotation are all broken

2 participants